Skip to content

feat: prometheus metrics endpoint on SN and DN nodes - #447

Open
joaopaulosr95 wants to merge 5 commits into
HDFGroup:masterfrom
joaopaulosr95:feat/prometheus-metrics
Open

feat: prometheus metrics endpoint on SN and DN nodes#447
joaopaulosr95 wants to merge 5 commits into
HDFGroup:masterfrom
joaopaulosr95:feat/prometheus-metrics

Conversation

@joaopaulosr95

@joaopaulosr95 joaopaulosr95 commented Jul 18, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Motivation

Running HSDS on Kubernetes, there's no machine-readable health signal beyond the /info JSON. This PR adds a Prometheus /metrics endpoint to every node, plus the Grafana dashboard, alert rules, and Kubernetes wiring to consume it — so operators can alert on health, saturation, and storage-backend problems with the usual Prometheus/Grafana stack.

What changed

/metrics endpoint (hsds/metrics.py), via the official prometheus_client:

  • aiohttp middleware records hsds_http_requests_total{method,status} and an hsds_http_request_duration_seconds histogram per request.
  • A scrape-time collector exposes readiness (hsds_node_ready), known DN count (hsds_active_dns), task saturation (hsds_tasks_active / hsds_tasks_max), WARN/ERROR log counts, LRU cache stats, and storage counters (errors, bytes read/written, by backend).
  • process_* / python_gc_* come free; /metrics is served even when the node isn't READY.

Async-internals instrumentation: SN→DN fan-out (hsds_internal_requests_total, hsds_internal_request_duration_seconds via a ClientSession TraceConfig), crawler saturation (hsds_crawler_queue_depth, hsds_crawler_active_workers), and housekeeping liveness/duration.

Storage stats bug fixes (found while validating metrics live):

  • fileClient._file_stats_increment was missing its final increment, so every POSIX storage counter stayed at zero.
  • bytes_in was never incremented; now recorded on get_object in the file, s3, and azure clients.

Kubernetes / deployment:

  • prometheus.io/scrape annotations and named container ports (sn, dn) on the deployment manifests.
  • Front HSDS with an Ingress or Gateway; restrict /metrics, /info, /about to in-cluster callers (403 at the edge) — example k8s_ingress_nginx.yml and k8s_gateway_envoy.yml. These metrics let a standard HPA/KEDA replace the custom 503 autoscaler.
  • Fixed the AWS deployment to declare its override configMap volume (previously mounted but never defined).

Observability tooling (admin/):

  • docker/docker-compose.metrics.yml: Prometheus + Grafana overlay for local runs (datasource pre-wired).
  • prometheus/prometheus.yml (example scrape, relabeled to the endpoint=sn|dn label the dashboard needs) and prometheus/alert.rules.yml.
  • grafana/hsds-dashboard.json: "HSDS" dashboard, 19 panels.

Docs: docs/prometheus_metrics.md (metric reference, rationale, annotation-vs-PodMonitor scraping, endpoint restriction, alerts, local stack, dashboard) and a README link.

Dependency: prometheus-client added to pyproject.toml / requirements.txt.

Testing

  • Unit tests in tests/unit/metrics_test.py (middleware, exposition, cache/storage/node/async metrics, handler, and a fileClient stats regression test); python testall.py passes; flake8 clean.
  • Verified live via the docker-compose stack: both SN and DN expose /metrics, Prometheus scrapes all targets with endpoint labels, promtool validates the config and rules, and all 19 dashboard panel queries resolve against a live Prometheus.

@joaopaulosr95

joaopaulosr95 commented Jul 18, 2026

Copy link
Copy Markdown
Contributor Author

@jreadey @mattjala flagging you both as you're the top contributors of this project. Yet independent of 446, this is also part of my attempt to make HSDS easier to operate.

@joaopaulosr95 joaopaulosr95 mentioned this pull request Jul 18, 2026
Merged
@jhendersonHDF
jhendersonHDF requested a review from jreadey July 21, 2026 16:02
@jreadey

jreadey commented Jul 24, 2026

Copy link
Copy Markdown
Member

Thanks for your contribution! Having a way to plug in tools like Grafana will be helpful for bother operators of the service and developers (e.g. to assist in determining where bottlenecks are).
I chatted a bit with gemini about the best approaches for adding metrics, and it agreed that prometheus is the best choice, but had some specific guidance (e.g. using PROMETHEUS_MULTIPROC_DIR) that you mind find helpful. You can find the conversation here: https://share.gemini.google/oPr74iW6dkkd.

Also, the original intent of the /info endpoint was to be the path for fetching metrics, but you added a /metrics path. Can /info just be re-purposed? Or would it be better to have just /metrics and deprecate /info?

Finally, how is(should) security handled? Some admins would prefer not to expose HSDS metric data to the outside world. Can the access be restricted to (say) just admin users?

@joaopaulosr95

joaopaulosr95 commented Jul 27, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@jreadey, thanks for the heads up.

I believe the multiproc concern doesn't apply to HSDS, as Gemini assumed we're using Gunicorn and firing multiple worker processes, which we're not. The hsds-servicenode and hsds-datanode entry points fire one process each; therefore, there's one single event loop per instance. So the multi-process intuition is actually multi-node.

About the dedicated /metrics endpoint: this is a general convention. When I was learning the code, the /info endpoint felt like a liveness/readiness probe, so it made sense to keep it separate. Please let me know if you want both concerns to live under the same endpoint, and I'll adjust it.

Regarding security, specific internal routes are usually hidden through custom snippets in the Ingress resource or HTTPRouteFilters if you're using the Gateway API. I'll add it to the docs with a working example.

@jreadey

jreadey commented Jul 28, 2026

Copy link
Copy Markdown
Member

Let's leave the /info path there for now -- I'll think about if it makes sense to remove or not after the merge.

Have you tired this update with Grafana (or do you plan to do so)? I guess if there are any potential issues that would be a good way to tease them out.

Should there be a prometheus.yml file as part of the PR?

@joaopaulosr95

joaopaulosr95 commented Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

Good callout! We're actually running a custom build from this fork in production for about a week alongside the proposed metrics. I'll add an example dashboard and update the docs about how to run it locally in a few.

@joaopaulosr95 @claude
feat: Prometheus/Grafana monitoring stack, dashboard, and metric fixes
7f4d0a6 
- admin/docker/docker-compose.metrics.yml: Prometheus + Grafana overlay
  for any HSDS compose stack (Prometheus datasource pre-wired)
- admin/prometheus/: example scrape config (relabels to the endpoint=sn|dn
  label the dashboard depends on) and promtool-checked alert rules
- admin/grafana/hsds-dashboard.json: "HSDS" dashboard, 19 panels across
  cluster-internals and async/background-job rows
- docs/prometheus_metrics.md + README: document the local stack, the
  endpoint-label dependency, and the dashboard; link the guide from the
  README
- rename metric hsds_active_dn_count -> hsds_active_dns (the _count suffix
  is reserved for counters per Prometheus convention)
- admin/kubernetes/k8s_deployment_aws.yml: declare the missing override
  configMap volume (kubectl apply would otherwise reject the manifest);
  drop a dead doc reference to a nonexistent singleton manifest

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@joaopaulosr95

joaopaulosr95 commented Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

@jreadey here's a screenshot of the Grafana dashboard that now lives in the admin/grafana folder
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jreadey jreadey Awaiting requested review from jreadey

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@joaopaulosr95 @jreadey